-
-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preserve current tenant through active job #319
Preserve current tenant through active job #319
Conversation
2b65594
to
1ec3d7b
Compare
…deserialization of an ActiveJob to preserve the current tenant.
1ec3d7b
to
ec8d88d
Compare
@excid3 any opinion on this? Looking to switch to GoodJob, would be cool to have this merged |
I like this. I think we'll want to undo the PR that automatically integrates with Sidekiq since this would double include the tenant in the params which doesn't need to happen. It'll still be useful for any jobs directly integrated with Sidekiq, so we will keep it around but this will be a more seamless experience for Rails. |
What is the recommended way to disable this? |
Not at the moment. Just remove that portion from your extension? |
This seems to be causing an interesting issue for me. I'm using SolidQueue and have an ActiveJob that has a retry configured:
This particular job is triggered at the start of a new trial which means that, rarely, the job runs before the "Organization" has been committed to the database. It just so happens that the Organization is also the tenant. The first thing I'm doing in my job is to query for the Organization:
So I would expect those rare cases to raise a RecordNotFound and the job to retry a few seconds later. However, it seems that it's raising prior to the job being performed when the GlobalID locater is called. Because of this order of events, the ActiveJob retry is not being engaged. I recognize that this exact situation is uncommon. Retrying on a RecordNotFound when the record that isn't found is the same as the tenant. I can find a workaround, possible enqueuing this one specific job inside of a I moved over to ActsAsTenant later in the life of this application so none of my jobs are making use of the current_tenant serialization. It would be convenient to be able to turn it off. Thanks for the gem. 🙏 |
@joshforbes Interesting race condition, and as you've mentioned you have work arounds. The problem seems to stem from GlobalId's docs for reference: https://github.com/rails/globalid Should be a reasonably easy change to the following if you want to create a PR: Be sure to add a test case: |
Add a new ActiveJobExtensions which hooks into the serialization and deserialization of an ActiveJob to preserve the current tenant through ActiveJob.
We are using this with GoodJob, though as it is hooking into ActiveJob I'd imagine it should work with any job processor which is ActiveJob compatible.